Skip to content

Remove PHP 7.1 from CI#3572

Closed
alanpoulain wants to merge 2 commits into
api-platform:2.5from
alanpoulain:ci-remove-php-7.1
Closed

Remove PHP 7.1 from CI#3572
alanpoulain wants to merge 2 commits into
api-platform:2.5from
alanpoulain:ci-remove-php-7.1

Conversation

@alanpoulain

Copy link
Copy Markdown
Member
Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tickets N/A
License MIT
Doc PR N/A

@alanpoulain

Copy link
Copy Markdown
Member Author

@dunglas could you remove 7.1 from required status check please?

@dunglas

dunglas commented May 20, 2020

Copy link
Copy Markdown
Member

But why? We still support PHP 7.1 isn't it?

@alanpoulain

Copy link
Copy Markdown
Member Author

We still support it, but it causes test issues because of the new Doctrine Inflector (BC changes and PHP > 7.1).
Also, I think it would be a good thing to drop it for 2.6 (because this version is not maintained anymore and some libs don't support it like the Doctrine ones).

@dunglas

dunglas commented May 22, 2020

Copy link
Copy Markdown
Member

Can't we add a BC layer to support older versions of Doctrine Inflector? Or aren't these versions maintained? That being said, OK for me to trop support for PHP 7.1 in 2.6, but we should keep the tests for 2.5.

Btw, at some point I would like to switch from Doctrine Inflector to Symfony String).

@alanpoulain

Copy link
Copy Markdown
Member Author

Yes, we can add a BC layer (that's what it has been done here: #3564).
However some tests cannot be kept (mainly because the plural of taxon was taxons before 1.4 and taxa after). That's why I wanted to remove PHP 7.1 from the tests but keep the compatibility anyway for API Platform 2.5. But not for 2.6.

@soyuka

soyuka commented May 26, 2020

Copy link
Copy Markdown
Member

let's rebase this onto master, wdyt @alanpoulain ?

@roukmoute

Copy link
Copy Markdown

Doctrine Inflector to Symfony String

Why @dunglas?

@dunglas

dunglas commented May 28, 2020

Copy link
Copy Markdown
Member

To prevent this kind of issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants